Skip to content

Conversation

@marghe-molaro
Copy link
Collaborator

This PR updates previous PR draft #1468

Now includes changes in the mother_and_newborn_info dataframe.

…th, label what is only used for ddebugging and will be later removed
…ible to all modules. For now add person_ID to the dict of info printed as the outer dictionary key logging seems to have a problem.
…lysis file now collects all relevant info and prints them
…rected analysis file such as for small number of cases where the DALYs are not explicitly resolved the average DALYs are still computed correctly [skip ci]
@tamuri
Copy link
Collaborator

tamuri commented Nov 22, 2025

If you want to keep the correct type for the value (instead of converting them to string and then having to convert them back) you can setup the different value columns for each of the types. i.e. the columns would be: entity, attribute, value_str, value_float, value_int, value_datetime etc. The default values for the entries would be NaN/None etc.

Or (slightly more complex logic), have different EAV dataframes (i.e. for us, the "key" for logging the entry) based on the type of value: EAV_str, EAV_float etc.

(I think the first way would be more sensible in your case)

@tamuri
Copy link
Collaborator

tamuri commented Nov 23, 2025

Don't worry about the linter errors. Looks like ruff has updated is having an episode with stuff that wasn't a problem yesterday. Feeling increasingly hostile towards ruff.

@marghe-molaro
Copy link
Collaborator Author

Don't worry about the linter errors. Looks like ruff has updated is having an episode with stuff that wasn't a problem yesterday. Feeling increasingly hostile towards ruff.

Join the club!! :)

- avoided using the more typical naming `event` or `signal` because they are already used.
@marghe-molaro marghe-molaro force-pushed the molaro/harvest-training-data-including-mni branch from a867458 to ebe0ebc Compare November 25, 2025 11:29
@marghe-molaro
Copy link
Collaborator Author

Now leveraging dispatcher introduced in PR #1759. All functions pertaining to collection of event chains are found in CollectEventChains module.

Still to fix:

  • Initialisation of parameters in new module
  • Module tests

# 1) generate_event_chains is set to True
# 2) the event belongs to modules of interest and
# 3) the event is not in the list of events to ignore
if p['generate_event_chains'] and (data['module'] in p['modules_of_interest']) and (data['link_info']['EventName'] not in p['events_to_ignore']):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tamuri, here is an example of checks taking place at the listener's end rather than the dispatcher's.
We could instead check whether it is worth dispatching the signal at all given relevant conditions.

This implementation is definitely tidier but, depending on how costly it is to dispatch/receive a signal, maybe not the most efficient

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I understand. This is the correct way to do it - the notifier will always broadcast, regardless. Are you intending to switch chain logging on and off during the simulation run? It could be that if the module has been registered, it logs.

For clarity, it's better if you exit early from the function if condition is not met, similar to our if not person.is_alive: return in other events.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and yes good point!

if self.generate_event_chains and (data['module'] in self.modules_of_interest) and (data['link_info']['EventName'] not in self.events_to_ignore):
if not self.generate_event_chains or (data['module'] not in self.modules_of_interest) or (data['link_info']['EventName'] in self.events_to_ignore):
return
else:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the else when the condition will exit the function. Just continue with what you want to happen in the main block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants